Skip to content

[fix] [broker] Internal reader of __change_events can not started after metadata store session rebuilt - #23018

Merged
Technoboy- merged 9 commits into
apache:masterfrom
poorbarcode:test/zk_session_expire
Jul 29, 2024
Merged

[fix] [broker] Internal reader of __change_events can not started after metadata store session rebuilt#23018
Technoboy- merged 9 commits into
apache:masterfrom
poorbarcode:test/zk_session_expire

Conversation

@poorbarcode

@poorbarcode poorbarcode commented Jul 9, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation & Modifications

1. Fix the issue below

  • settings: enable systemTopic & topicLevelPolicies.
  • Own topics
    • broker-2 loads a topic-a up.
    • broker-2 loads system topic({ns}/__change_events) up.
  • The ZK session expires.
  • Bundle shedding
    • topic-a was transferred to broker-0
    • the system topic({ns}/__change_events) was transferred to broker-0
  • (Highlight) Issue occurs on broker-2
    • The internal reader for {ns}/__change_events lookup the topic's owner
    • broker-2 responds broker-2 due to its OwnershipCache contains the old value.
    • The internal reader for {ns}/__change_events can not be created successfully, all topics under this namespace can not work.
    • The consumers/producers who call lookup to broker-2 will get a wrong value, and keep getting a "Please redo the lookup" error.

2. Add a test to confirm the topic will be unloaded after the metadata store session is rebuilt.

  • Broker load a topic up.
  • The ZK session expires.
  • The ZK session rebuild.
  • Verify: The topics that the broker owned will be unloaded

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode added this to the 3.4.0 milestone Jul 9, 2024
@poorbarcode
poorbarcode requested a review from merlimat July 9, 2024 17:40
@poorbarcode poorbarcode self-assigned this Jul 9, 2024
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jul 9, 2024
@poorbarcode poorbarcode mentioned this pull request Jul 9, 2024
Closed
4 tasks
@codecov-commenter

codecov-commenter commented Jul 9, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.45%. Comparing base (bbc6224) to head (e439b97).
⚠️ Report is 1230 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23018      +/-   ##
============================================
- Coverage     73.57%   73.45%   -0.13%     
- Complexity    32624    33216     +592     
============================================
  Files          1877     1917      +40     
  Lines        139502   144072    +4570     
  Branches      15299    15743     +444     
============================================
+ Hits         102638   105824    +3186     
- Misses        28908    30131    +1223     
- Partials       7956     8117     +161
Flag Coverage Δ
inttests 27.73% <80.00%> (+3.14%) ⬆️
systests 24.72% <0.00%> (+0.40%) ⬆️
unittests 72.52% <100.00%> (-0.33%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...dbalance/extensions/ExtensibleLoadManagerImpl.java 79.45% <ø> (-0.64%) ⬇️
...roker/loadbalance/impl/ModularLoadManagerImpl.java 84.98% <ø> (+<0.01%) ⬆️
...he/pulsar/metadata/impl/AbstractMetadataStore.java 84.94% <100.00%> (+0.38%) ⬆️

... and 509 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@heesung-sohn

heesung-sohn commented Jul 9, 2024

Copy link
Copy Markdown
Contributor

Can we cover the new load balancer behavior too?

@shibd
shibd self-requested a review July 10, 2024 02:54
@poorbarcode

poorbarcode commented Jul 10, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@heesung-sn

Can we cover the new load balancer behavior too?

Sure, I will push two following PRs to do this:

  • Enable system topic and topic-level policies. So far, this test can not pass when enabled system topic, I am checking why it does not work, and I will push a fix if there is an issue. Added into the current PR.
  • Enable ExtensibleLoadManager, since it depends on the system topic, this test will be added after the above one.

@poorbarcode
poorbarcode requested a review from heesung-sohn July 10, 2024 03:06
@poorbarcode poorbarcode changed the title [improve] [test] Add a test: topic will be unloaded after metadata store session rebuilt [fix] [broker] Topic should be unloaded after metadata store session expired Jul 10, 2024
@poorbarcode poorbarcode changed the title [fix] [broker] Topic should be unloaded after metadata store session expired [fix] [broker] Internal reader of __change_events can not started after metadata store session rebuilt Jul 10, 2024
@poorbarcode
poorbarcode requested review from merlimat and removed request for merlimat July 10, 2024 13:06
@poorbarcode
poorbarcode force-pushed the test/zk_session_expire branch from 68143c4 to bf8b6a9 Compare July 26, 2024 00:33
lhotari
lhotari reviewed Jul 29, 2024
View reviewed changes
Comment thread pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/AbstractMetadataStore.java Outdated
@poorbarcode
poorbarcode requested a review from lhotari July 29, 2024 10:11
@Technoboy-
Technoboy- merged commit b955c65 into apache:master Jul 29, 2024
poorbarcode added a commit that referenced this pull request Jul 30, 2024
@poorbarcode
[fix] [broker] Internal reader of __change_events can not started aft…
0e59678 
…er metadata store session rebuilt (#23018)

(cherry picked from commit b955c65)
poorbarcode added a commit that referenced this pull request Jul 30, 2024
@poorbarcode
[fix] [broker] Internal reader of __change_events can not started aft…
ae2b4a1 
…er metadata store session rebuilt (#23018)

(cherry picked from commit b955c65)
poorbarcode added a commit that referenced this pull request Jul 30, 2024
@poorbarcode
[fix] [broker] Internal reader of __change_events can not started aft…
e3e6849 
…er metadata store session rebuilt (#23018)

(cherry picked from commit b955c65)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 30, 2024
@poorbarcode @nikhil-ctds
[fix] [broker] Internal reader of __change_events can not started aft…
ce5fb3a 
…er metadata store session rebuilt (apache#23018)

(cherry picked from commit b955c65)
(cherry picked from commit 0e59678)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 30, 2024
@poorbarcode @srinath-ctds
[fix] [broker] Internal reader of __change_events can not started aft…
148077c 
…er metadata store session rebuilt (apache#23018)

(cherry picked from commit b955c65)
(cherry picked from commit 0e59678)
@poorbarcode poorbarcode mentioned this pull request Sep 25, 2024
Closed
4 tasks
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Feb 6, 2025
@poorbarcode @nodece
[fix] [broker] Internal reader of __change_events can not started aft…
37795fe 
…er metadata store session rebuilt (apache#23018)

(cherry picked from commit b955c65)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
@poorbarcode @hanmz
[fix] [broker] Internal reader of __change_events can not started aft…
1d10181 
…er metadata store session rebuilt (apache#23018)
This was referenced Aug 27, 2025
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhotari lhotari lhotari approved these changes
@Technoboy- Technoboy- Technoboy- approved these changes
@shibd shibd shibd approved these changes
@codelipenghui codelipenghui Awaiting requested review from codelipenghui
@heesung-sohn heesung-sohn Awaiting requested review from heesung-sohn
@merlimat merlimat Awaiting requested review from merlimat

Assignees

@poorbarcode poorbarcode

Labels

cherry-picked/branch-3.0 cherry-picked/branch-3.2 cherry-picked/branch-3.3 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.7 release/3.2.5 release/3.3.2

Projects

None yet

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

7 participants

@poorbarcode @codecov-commenter @heesung-sohn @merlimat @lhotari @Technoboy- @shibd